Add e2e test case for default scaling strategy #6419
Conversation
Signed-off-by: June Han <[email protected]>
![semgrep-app[bot]](https://avatars.githubusercontent.com/in/60555?s=60&v=4){kind=small}
| password = fmt.Sprintf("%s-password", testName) | ||
| vhost = "/" | ||
| connectionString = fmt.Sprintf("amqp://%s:%s@rabbitmq.%s.svc.cluster.local/", user, password, rmqNamespace) | ||
| httpConnectionString = fmt.Sprintf("http://%s:%s@rabbitmq.%s.svc.cluster.local/", user, password, rmqNamespace) |
There was a problem hiding this comment.
Semgrep found a possible database connection string built with string concatenation. Check for proper encoding/escaping of components to prevent parse errors and injection vulnerabilities.
Ignore this finding from db-connection-string.
tests/internals/scaling_strategies/default_scaling_strategy_test.go
Outdated
Show resolved
Hide resolved
|
Please run the e2e test task! Thanks! |
|
@junekhan I see you've committed the test I sent you from this conversation I can send you the output if that saves having to create an entire PR just to run a test |
Sorry, I don't have an environment to run this test. And I think it's nontrivial to add the test case, otherwise, there wouldn't have been any controversy. It can serve as evidence for people holding the same idea as you. |
Signed-off-by: June Han <[email protected]>
| password = fmt.Sprintf("%s-password", testName) | ||
| vhost = "/" | ||
| connectionString = fmt.Sprintf("amqp://%s:%s@rabbitmq.%s.svc.cluster.local/", user, password, rmqNamespace) | ||
| httpConnectionString = fmt.Sprintf("http://%s:%s@rabbitmq.%s.svc.cluster.local/", user, password, rmqNamespace) |
There was a problem hiding this comment.
Semgrep found a possible database connection string built with string concatenation. Check for proper encoding/escaping of components to prevent parse errors and injection vulnerabilities.
Ignore this finding from db-connection-string.
| user = fmt.Sprintf("%s-user", testName) | ||
| password = fmt.Sprintf("%s-password", testName) | ||
| vhost = "/" | ||
| connectionString = fmt.Sprintf("amqp://%s:%s@rabbitmq.%s.svc.cluster.local/", user, password, rmqNamespace) |
There was a problem hiding this comment.
Semgrep found a possible database connection string built with string concatenation. Check for proper encoding/escaping of components to prevent parse errors and injection vulnerabilities.
Ignore this finding from db-connection-string.
I created an environment to run the tests. Regardless, here is the output if you want to see. I don't disagree that adding a test would be useful for the project, though I did not intend this one to cover all necessary test cases, just demonstrate your example. I suppose something is better than nothing. I am a little surprised you would commit someone else's code without checking, even if that code is just a reworking of existing code, that is what most of the tests are. But not a point I want to belabour. Anyway, I assume you have not been able to run the tests I sent for |
@chinery Apologies for not asking for your permission of the test snippet. If you agree it's much simpler for people to follow the test case and the outcome than to read lengthy passages and reason in mind, you can re-auther and create another pull request. I will immediately withdraw this PR. |
|
/run-e2e internal* |
Add e2e test case for default scaling strategy
Checklist
Fixes #6416
Relates to #